Dedicated SV copying code in place of Perl_sv_setsv_flags by richardleach · Pull Request #23202 · Perl/perl5

richardleach · 2025-04-15T23:29:12Z

Perl_sv_setsv_flags is the heavyweight function for assigning the value(s) of
a source SV to a destination SV. It contains many branches for preparing the
destination SV prior to assignment. However:

If the destination SV has just been created, much of that logic isn't needed.
When cloning a SV, simple assignments (particularly IVs and PVs) dominate.

This set of commits:

Extracts the "is this CoWable?" test from Perl_sv_setsv_flags into a macro.
Adds Perl_sv_freshcopy_flags and two static helper functions.
Modifies Perl_newSVsv_flags and Perl_sv_mortalcopy_flags to use them.
Standardizes a number of call sites that did their own things but really
should use Perl_newSVsv_flags or Perl_sv_mortalcopy_flags.

Using perl's test harness as a guide:

Bodyless code handles 45% of calls to Perl_newSVsv_flags and
57% of calls to Perl_sv_mortalcopy_flags.
The SVt_PV/SVp_POK code handles 32% of calls to
Perl_newSVsv_flags and 36% of calls to Perl_sv_mortalcopy_flags.
S_sv_freshcopy_flags code handles 95% of the remainder in
Perl_newSVsv_flags and 91% of the remainder in to Perl_sv_mortalcopy_flags.

With these changes compared with a build of blead:

perl -e 'for (1..100_000) { my $x = [ (1) x 1000 ]; }' runs 10% faster
perl -e 'for (1..100_000) { my $x = [ ("Perl") x 250 ]; }' runs 45% faster

This set of changes does require a perldelta entry and has one.

pp.c

pp_hot.c

sv.c

Leont · 2025-04-29T20:52:21Z

Cloning is rather unfortunate choice of words, given that it has a very specific meaning in our codebase that is quite different from what this PR is about. Renaming the PR may be helpful.

richardleach · 2025-05-08T17:06:30Z

I've made a lot of changes following earlier comments - thanks for those - and have finally force-pushed.

These changes aren't complete. For example:

Measured performance seems worse than in the PR version, so I need to look into that
Might change sflag handling/ SvFLAGS(dsv) setting
Not settled on struct membet initialisation
Might still rename the function that is currently Perl_newSVsv_flags and have newSVsv_flags be a macro that checks (ssv) before calling the sv.c function.

sv.c

tonycoz · 2025-08-13T01:40:54Z

the last commit's message indicates it should be squashed

sv.c

richardleach · 2025-08-13T20:52:34Z

Thanks, I've hopefully addressed those comments and squashed all trailing commits.

sv.c

Perl_newSVsv_flags_NN creates a fresh SV that contains the values of its source SV argument. It's like calling `new_SV(dsv)` followed by `sv_setsv_flags(dsv, ssv, flags`, but is optimized for a brand new destination SV and the most common code paths. The intended initial users for this new function were: * Perl_sv_mortalcopy_flags (still in sv.c) * Perl_newSVsv_flags (now a simple function in sv_inline.h) Perl_newSVsv_flags_NN prioritises the following hot cases: * SVt_IV containing an IV * SVt_IV containing an RV * SVt_NV containing an NV * SVt_PV containing a PV It will then check for: * SVt_NULL * SVt_IV containing a UV * SVt_LAST The helper function S_newSVsv_flags_NN_PVxx is called for everything else. It will use Perl_sv_setsv_flags as a fallback for rare or tricky cases. S_newSVsv_flags_NN_POK is a dedicated helper for string swipe/COW/copy logic and is called from both Perl_newSVsv_flags_NN and S_newSVsv_flags_NN_PVxx. With these changes compared with the previous commit: * `perl -e 'for (1..100_000_0) { my $x = { (1) x 1000 }; }'` runs about 20% faster * `perl -e 'for (1..100_000_0) { my $x = { ("Perl") x 250 }' runs about 40% faster * `perl -e 'for (1..100_000_0) { my $x = { a => 1, b => 2, c => 3, d => 4, e => 5 }; }'` is a touch faster, but within the margin for error * `perl -e 'for (1..100_000_0) { my $x = { a => "Perl", b => "Perl", c => "Perl", d => "Perl", e => "Perl" } ; }'` runs about 17% faster

Perl_newSVsv_flags has become just a stub around Perl_newSVsv_flags_NN. For callers where the source SV* is NULL, not having to call a function in sv.c to immediately return is very desirable.

Besides using the just-introduced faster path for SV copying, this allows the check for SV_GMAGIC to be pushed into the called function without having to worry about SV leaks. Two additional micro-optimizations are also in this commit: * A pointer to xav_fill is cached for use in the loop. This can be used directly to update AvFILLp(av), rather than having to get there from av's SV* each time. * The value of the loop iterator, i, is directly written into xav_fill, rather than getting the value in that slot, incrementing it (to get the same value as i), and writing it back.

richardleach · 2025-08-23T16:45:41Z

Many thanks for all the reviews @tonycoz & @bulk88 - it's a relief to finally get this in before the github-actions bot adds the hasConflicts flag again!

richardleach added the defer-next-dev This PR should not be merged yet, but await the next development cycle label Apr 15, 2025